Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

capacity: add benchmark for CSIStorageCapacity update #706

Closed
wants to merge 1 commit into from

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Feb 18, 2022

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

It may be possible to optimize updating capacity by using patching. Before
trying that we need a benchmark.

Which issue(s) this PR fixes:

Fixes #467

Special notes for your reviewer:

This benchmark acts as a client with basically unlimited local rate
limits. When run long enough, the results are stable:

$ go test -run=xxx -bench=. -count=5 -benchtime=1m . | tee /tmp/log
...
$ $GOPATH/bin/benchstat /tmp/log
name               time/op
CapacityUpdate-36  3.93ms ± 1%

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Feb 18, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pohly
To complete the pull request process, please assign msau42 after the PR has been reviewed.
You can assign the PR to them by writing /assign @msau42 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 18, 2022
It may be possible to optimize updating capacity by using patching. Before
trying that we need a benchmark.

This benchmark acts as a client with basically unlimited local rate
limits. When run long enough, the results are stable:

$ go test -run=xxx -bench=. -count=5 -benchtime=1m . | tee /tmp/log
...
$ $GOPATH/bin/benchstat /tmp/log
name               time/op
CapacityUpdate-36  3.93ms ± 1%
@pohly pohly force-pushed the capacity-update branch from 1beacdd to 08bc33c Compare April 25, 2022 17:57
@k8s-ci-robot
Copy link
Contributor

@pohly: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 27, 2022
@pohly
Copy link
Contributor Author

pohly commented May 8, 2022

There doesn't seem to be enough interest in having these benchmarks included in the repo.

/close

@k8s-ci-robot
Copy link
Contributor

@pohly: Closed this PR.

In response to this:

There doesn't seem to be enough interest in having these benchmarks included in the repo.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

storage capacity: patch vs. update of CSIStorageCapacity
2 participants